Skip to content

CLIM-1322 (4): Popup Title Display Discrepancy When InteractiveRegionOption.GRIDDED_DATA#685

Merged
renoirb merged 9 commits into
mainfrom
CLIM-1322_4_Popup-Title-Name-Resolution
May 14, 2026
Merged

CLIM-1322 (4): Popup Title Display Discrepancy When InteractiveRegionOption.GRIDDED_DATA#685
renoirb merged 9 commits into
mainfrom
CLIM-1322_4_Popup-Title-Name-Resolution

Conversation

@renoirb
Copy link
Copy Markdown
Contributor

@renoirb renoirb commented May 9, 2026

Popup Title Display Discrepancy When InteractiveRegionOption.GRIDDED_DATA

Summary

Pass 4 minimal hot-fix synthesis for CLIM-1322. Cherry-picks the load-bearing fixes from Passes 2 and 3, adds two native commits (Atoms 15 & 16) to close DoD-1 (name parity EN/FR for UC-Search) and DoD-2 (raw lat/lng pin precision for UC-RawCoordPaste) within InteractiveRegionOption.GRIDDED_DATA. Polygon modes (CENSUS / HEALTH / WATERSHED) untouched. No SQL filter rewrite — that work remains parked on CLIM-1322_3_Popup-Title-Name-Resolution for a future ticket.

Branched from origin/main at 2044c88d. Diff vs main: 9 files, ~140 added / ~21 removed.

Use cases addressed (GRIDDED_DATA)

Use case What changes in this PR
UC-Search (autocomplete pick) Popup title built from the autocomplete row directly: ${item.text}, ${item.province_short} (e.g. "Montréal, QC"). One API call: /wp-json/cdc/v2/location_search?q=…. No second /get_location_by_coords round-trip. Closes DoD-1.
UC-MapClick Untouched — real DOM clicks; existing handleClick flow.
UC-RawCoordPaste (typed lat, lng) Marker now lands at the typed coord exactly — bypasses leaflet-search's showLocation → moveToLocation → handleLocationChange → dispatchMapClick → containerPointToLatLng chain that previously produced ~80m drift. Polygon modes preserve existing behavior (drift benign at polygon scale; title from layer.properties.label_* regardless). Closes DoD-2.

Atoms

Atom Origin What it does
Atom 1 cherry-picked from #682 Server: cdc_get_location_by_coords SELECT aliases province_fr as province. Eliminates FR Undefined array key "province" warning and the trailing-comma title ("Sainte-Adèle, ""Sainte-Adèle, QC").
Atom 3 cherry-picked from #682 Client: locationNotFound passes the readable .title to showLocation's second arg instead of the opaque 5-char .geo_id.
Atom 14 cherry-picked from CLIM-1322_3_… Introduces selectGriddedLocation({latlng, title}) exported from useMapInteractions; prop-chained as onSelectGriddedLocation through Map → MapContainer → SearchControl. UC-Search GRIDDED invokes it directly with the autocomplete row's data — no synthetic-click round-trip.
Atom 12 cherry-picked from CLIM-1322_3_… Server: cdc_location_search additively returns province_short via the same short_province() helper used by cdc_get_location_by_coords. Client: SearchControlLocationItem.province_short typed; UC-Search popup title becomes the short-form ${item.text}, ${item.province_short}.
Atom 15 native dispatchMapClick drops its latlng parameter — it was always discarded (void latlng;). Clarifies design intent: the function is solely about dispatching a real-browser-equivalent PointerEvent('click') at the map container's geometric center after setView settles. Updates two call sites (search-control.tsx, recent-locations.tsx).
Atom 16 native UC-RawCoordPaste GRIDDED early-return in locationNotFound: branches on interactiveRegion === GRIDDED_DATA && onSelectGriddedLocation to call setView + onSelectGriddedLocation({latlng: typed, title: server-resolved}) + return. Bypasses leaflet-search's showLocation chain. Polygon-mode else branch preserves the existing showLocation + spread-override path.

Definition of Done

  • DoD-1 — name parity EN/FR for UC-Search at the same row: achieved structurally. Same autocomplete row → same province_short → same short-form popup title regardless of UI language. Example: Laval row EGYIC"Laval, QC" whether the user typed in EN or FR autocomplete.
  • DoD-2 — raw lat/lng pin precision: achieved empirically. Atom 16's early-return passes the typed latLng directly to selectGriddedLocation, bypassing the synthetic-click reprojection chain that produced ~80m drift via containerPointToLatLng on container-center pixel coords.

Out of scope (parked for follow-up)

  • SQL filter rewrite / gen_term tier preferences — full design and probe-verification on CLIM-1322_3_Popup-Title-Name-Resolution (not merged). Would address T28 (Park-class defects: McMasterville → "Parc Ensoleillé", Bedford → "Parc des Bouts-de-Choux") for UC-MapClick and UC-LocateMe. UC-Search T28 dissolves structurally in this PR via Atom 14 because the autocomplete row never round-trips through cdc_get_location_by_coords.
  • dispatchMapClick → moveAndPointAt rename + per-map WeakMap precise-latlng channel — Iteration 2's Atoms 6/8/9. Out of scope here because UC-Search bypasses the synthetic-click pipeline (no consumer in scope) and UC-RawCoordPaste GRIDDED takes the early-return path in Atom 16. Logically a single follow-up unit if a future ticket needs precise pin placement under the synthetic-click path.
  • MapFeatureProperties typing migration — parked on a separate local branch stashed/MapFeatureProperties (single pure-typing commit). Pure refactor; pick up in a typing-only PR.

Verification

  • ./dev.sh typecheck-apps clean.
  • ./dev.sh lint-apps clean on touched files (one pre-existing warning on an untouched line in Map's useEffect deps).
  • API probes against local development dev-en and dev-fr for the 16-anchor reference table (Vancouver / Montréal / Halifax / Sainte-Adèle / Toronto / Edmonton / Calgary / Victoria / Ottawa / Québec / Charlottetown / St. John's / Saskatoon / Banff / Bedford / McMasterville) confirm short-form popup titles for UC-Search GRIDDED.
  • Check with raw lat,lon raw coord tuples (UC-RawCoordPaste) at five precision levels (2 to 6 decimal places) confirmed marker matches typed coord after Atom16 — the empirical signature that previously identified the ~80m drift (identical post-setView viewport-center across coords differing only in low-order decimals) is gone.

Related

Supersedes earlier Passes:

@renoirb renoirb changed the title Clim 1322 4 popup title name resolution CLIM-1322 (4): Popup Title Display Discrepancy When InteractiveRegionOption.GRIDDED_DATA May 9, 2026
@renoirb renoirb force-pushed the CLIM-1322_4_Popup-Title-Name-Resolution branch from b1dc7e7 to c6a07ac Compare May 11, 2026 19:08
renoirb added 3 commits May 11, 2026 15:55
The function selected "province_fr" (or "province") via $term_append but
the result row was always read as $result['province']. In FR this produced
"Undefined array key" warnings and a null province_short, which in turn
yielded titles like "Sainte-Adèle, " (trailing comma).

Aliases the column back to the "province" key, matching the pattern
already used by cdc_get_location_by_id() at the same file.
In the locationNotFound branch (raw lat,lng paste), showLocation()'s
title argument was the opaque 5-char geo_id (e.g. "EPYTF", "EJK8O"),
producing a popup title users could not read.

The fetched response already carries a human-readable title field built
server-side; pass that instead.
@renoirb renoirb force-pushed the CLIM-1322_4_Popup-Title-Name-Resolution branch 2 times, most recently from c039d6b to 42582e8 Compare May 12, 2026 07:34
@renoirb renoirb force-pushed the CLIM-1322_4_Popup-Title-Name-Resolution branch from 42582e8 to 672e7ef Compare May 12, 2026 07:40
@renoirb renoirb requested a review from ChaamC May 12, 2026 12:20
@renoirb renoirb marked this pull request as ready for review May 12, 2026 12:21
Comment on lines 282 to 300
// Check if the coordinates are valid if the location is empty.
const latLng = parseLatLon(this._input.value);
const latLon = parseLatLon(this._input.value);
// If the coordinates are valid, move to that location.
if (latLng && !latLng.isPartial) {
// Fetch location data
const locationByCoords = await fetchLocationByCoords({ lat: latLng.lat, lng: latLng.lon });
// Trigger show location.
this.showLocation(locationByCoords, locationByCoords.geo_id);
if (latLon && !latLon.isPartial) {
const latLng = {
lat: latLon.lat,
lng: latLon.lon,
}
const locationByCoords = await fetchLocationByCoords(latLng);
const locationAtTypedCoords = {
...locationByCoords,
lat: latLng.lat,
lng: latLng.lng,
};
this.showLocation(
locationAtTypedCoords,
locationByCoords.title,
);
}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make the map move to the entered coordinates.

Comment on lines +140 to +143
onSelectGriddedLocation({
latlng,
title: autocompleteItem.title,
});
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have the impression that's what opens up the PopUp thing. And the lat,lon at.

renoirb added 4 commits May 12, 2026 16:44
…e province_short in autocomplete

Atom 9-equivalent's buildLocationTitle() produced the verbose autocomplete
dropdown display ("Montréal, (Ville), Montréal; Montréal, Québec") in the
popup title; make UC-Search's popup match the server-resolved short shape
("Montréal, QC") that cdc_get_location_by_coords already returns.

Server-side cdc_location_search() now adds province_short to each item,
derived via the existing short_province() helper from fw-child/functions.php
(already used by cdc_get_location_by_coords and cdc_get_location_by_id), so
the client doesn't need its own province-name mapping. buildLocationTitle()
is unchanged — leaflet-search still uses it as the dropdown display key.
Empirical drift in UC-RawCoordPaste was traced to leaflet-search's
showLocation synchronously invoking moveToLocation -> handleLocationChange
-> dispatchMapClick (container-center synthetic click) -> drifted ~80m
re-projection in handleClick's addMarker. This commit bypasses that chain
in GRIDDED mode by calling setView + onSelectGriddedLocation directly with
the typed lat/lng, mirroring the same-intent-same-pattern path UC-Search
GRIDDED already uses. Polygon modes (CENSUS/HEALTH/WATERSHED) preserve the
existing showLocation path because their popup title comes from
layer.properties.label_* via handleClick's override and the ~80m drift is
benign at polygon scale.
@renoirb renoirb force-pushed the CLIM-1322_4_Popup-Title-Name-Resolution branch from 672e7ef to 3b6bc0b Compare May 12, 2026 20:46
@renoirb renoirb self-assigned this May 12, 2026
Comment on lines +292 to +311
// Bypass leaflet-search's showLocation pipeline for GRIDDED mode so the
// marker lands at the typed coordinates rather than the synthetic-click
// container-center reprojection. Polygon modes (CENSUS/HEALTH/WATERSHED)
// fall through to the existing showLocation path because their popup
// title comes from layer.properties.label_* and the small drift is
// benign at polygon scale.
const interactiveRegion = climateVariable?.getInteractiveRegion()
?? InteractiveRegionOption.GRIDDED_DATA;

if (
interactiveRegion === InteractiveRegionOption.GRIDDED_DATA
&& onSelectGriddedLocation
) {
map.setView(latLng, SEARCH_DEFAULT_ZOOM);
onSelectGriddedLocation({
latlng: L.latLng(latLng.lat, latLng.lng),
title: locationByCoords.title,
});
return;
}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically the same pattern as when selecting one of the autocomplete suggestion, but this time for when it's raw coordinates.

@renoirb renoirb force-pushed the CLIM-1322_4_Popup-Title-Name-Resolution branch from 57d10a9 to d5f0fd3 Compare May 13, 2026 12:43
@renoirb renoirb merged commit f05ea85 into main May 14, 2026
@renoirb renoirb deleted the CLIM-1322_4_Popup-Title-Name-Resolution branch May 14, 2026 03:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants